fix(docker): set fixed MACs on network endpoints#2619
Merged
Conversation
Purpose of the change: - Store fixed Docker MAC addresses as first-class template network settings. - Emit Docker's endpoint-level mac-address option when the GUI owns network creation. How behavior was before: - Users had to put --mac-address in Extra Parameters. - That used the legacy container-level flag and could force slow container rebuild behavior. - Docker service network restore only treated ExtraParams MACs as the custom-primary rebuild path. Why that was a problem: - Docker 28+ generates random MACs by default, which floods routers with stale entries after restarts. - The existing workaround made Docker service startup slow for affected containers. What the new change accomplishes: - Adds a Fixed MAC address field beside Fixed IP address. - Migrates eligible --mac-address values out of Extra Parameters into <MyMAC>. - Preserves ExtraParams-owned networking when --network/--net is already specified there. - Restores custom network endpoints with the stored template MAC without triggering the legacy rebuild path. How it works: - Builds --network=name=...,ip=...,mac-address=... for GUI-owned custom networks. - Removes the legacy --mac-address only when the replacement endpoint option is emitted. - Uses <MyMAC> during rc.docker network restore before falling back to inspect or legacy ExtraParams.
Contributor
WalkthroughAdds fixed-container MAC support across UI, server, and runtime: new helptext key, UI field and client-side normalization/validation, server-side MAC extraction/normalization and command synthesis (mac-address via Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Browser as Browser/UI
participant Server as Server
participant Docker as Docker Runtime
User->>Browser: Fill container form (contMyMAC, ExtraParams)
Browser->>Browser: prepareConfig(): normalize MAC (trim, -→:, lowercase), detect --net/--network, validate
alt Validation fails
Browser->>User: Block submit, show alert
else Validation passes
Browser->>Server: POST form (contMyMAC, ExtraParams)
Server->>Server: postToXML()/xmlToVar(): derive MyMAC or extract from ExtraParams, remove legacy --mac-address if applicable
Server->>Server: xmlToCommand(): suppress standalone --mac-address, synthesize --network(...,mac-address=...) when appropriate
Server->>Docker: Create/update container via API/CLI with generated command/config
Docker-->>Server: Return status
Server-->>Browser: Respond success/failure
Browser-->>User: Display result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
emhttp/plugins/dynamix.docker.manager/include/Helpers.php (1)
95-108:⚠️ Potential issue | 🟠 MajorHandle both
--net(work)=valueand--net(work) valuesyntax forms.Docker CLI accepts both
--network=fooand--network foo, but the current regex pattern only matches the equals-sign form. If a template or user-supplied extra parameters use the space-delimited syntax,$extraNetworkis incorrectly set to false, causingpostToXML()to wrongly migrate--mac-addressparameters out of ExtraParams andxmlToCommand()to append a second GUI-managed network flag instead of respecting the user override. This breaks the "preserve ExtraParams-owned networking" logic.🔧 Suggested fix
+function hasNetworkOverrideParam($extraParams) { + return is_string($extraParams) + && preg_match('/(?:^|\s)--net(?:work)?(?:=|\s+)/', $extraParams); +} + function postToXML($post, $setOwnership=false) { @@ - $extraNetwork = preg_match('/\-\-net(work)?=/', $post['contExtraParams'] ?? ''); + $extraNetwork = hasNetworkOverrideParam($post['contExtraParams'] ?? ''); @@ function xmlToCommand($xml, $create_paths=false) { @@ - $extraNetwork = preg_match('/\-\-net(work)?=/',$xml['ExtraParams']); + $extraNetwork = hasNetworkOverrideParam($xml['ExtraParams']);Also applies to: line 349
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix.docker.manager/include/Helpers.php` around lines 95 - 108, The regex used to detect a user-supplied network flag only matches the equals form, so update the detection logic for $extraNetwork to accept both “--net(work)=value” and “--net(work) value” (e.g. allow either “=” or whitespace after the flag) so extractMacAddressParam/removeMacAddressParam and the postToXML()/xmlToCommand() flow correctly treat ExtraParams-owned networking; apply the same regex change to the other occurrence referenced (the code at line ~349) so both checks handle the space-delimited syntax.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@emhttp/plugins/dynamix.docker.manager/include/Helpers.php`:
- Around line 95-108: The regex used to detect a user-supplied network flag only
matches the equals form, so update the detection logic for $extraNetwork to
accept both “--net(work)=value” and “--net(work) value” (e.g. allow either “=”
or whitespace after the flag) so extractMacAddressParam/removeMacAddressParam
and the postToXML()/xmlToCommand() flow correctly treat ExtraParams-owned
networking; apply the same regex change to the other occurrence referenced (the
code at line ~349) so both checks handle the space-delimited syntax.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 23b079cc-5db9-455b-a5fe-9852e8c48569
📒 Files selected for processing (4)
emhttp/languages/en_US/helptext.txtemhttp/plugins/dynamix.docker.manager/include/CreateDocker.phpemhttp/plugins/dynamix.docker.manager/include/Helpers.phpetc/rc.d/rc.docker
Purpose of the change: - Reject invalid fixed MAC addresses before Docker tries to create container networking. - Normalize valid fixed MAC values to colon-separated lowercase form. How behavior was before: - The form accepted MAC values such as 61-F2-9A-0D-7E-C0. - Docker later failed because MACs with an odd first octet are multicast addresses and cannot be assigned to container interfaces. Why that was a problem: - Users saw a low-level Docker networking error after submitting the form. - Hyphenated or compact valid MAC values were not normalized before being stored. What the new change accomplishes: - Adds client-side and server-side validation for unicast MAC addresses. - Updates help text with an example valid MAC address. - Keeps ExtraParams-owned network settings as the source of truth when --network or --net is specified there. How it works: - Accepts colon, hyphen, or compact hexadecimal MAC input. - Normalizes valid values to colon-separated lowercase format. - Requires the first octet to be even so multicast MACs are rejected.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php (1)
753-759: Add the same ExtraParams-network guard on the client for immediate feedback.Line 753+ validates MAC format, but users still hit a server round-trip when
contMyMACis set while Extra Parameters already own networking. Mirror the server check inprepareConfig(...)for better UX.💡 Suggested client-side parity check
function prepareConfig(form) { var types = [], values = [], targets = [], vcpu = []; + var extraParams = ($(form).find('input[name="contExtraParams"]').val() || ''); + var extraNetwork = /(^|\s)--net(work)?(?:=|\s+)/.test(extraParams); var myMAC = $(form).find('input[name="contMyMAC"]').val().trim().replaceAll('-', ':').toLowerCase(); + if (extraNetwork && myMAC) { + swal({title:"_(Invalid network/MAC combination)_",text:"_(Fixed MAC address cannot be used when Extra Parameters specify --network or --net. Add mac-address to the Extra Parameters network option instead.)_",type:"error",html:true}); + return false; + } if (myMAC && !isValidUnicastMacAddress(myMAC)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php` around lines 753 - 759, The client-side form validation currently checks contMyMAC format but doesn't prevent submission when Extra Params already specify networking; add a parity check in the same prepareConfig(...) client path: read the form's contExtraParams (or equivalent extra params input) and if contMyMAC is non-empty and contExtraParams contains any network option (e.g. “--network” or “--net”/“-p” style network settings or a literal “host”), show the same swal error message and return false so the user gets immediate feedback instead of a server round-trip. Ensure you reference the existing contMyMAC handling and place this guard before form submission in prepareConfig(...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php`:
- Around line 72-73: The check that sets $extraNetwork only matches
`--net=`/`--network=` and misses space-separated forms (e.g. `--net bridge`), so
change the regex that inspects $_POST['contExtraParams'] in CreateDocker.php to
detect both syntaxes; update the pattern used for $extraNetwork (and any related
checks) to match `--net` or `--network` followed by either `=` or whitespace
(e.g. use a regex like /(?:--net(?:work)?)(?:=|\s+)/) so the guard around
$_POST['contMyMAC'] cannot be bypassed by space-separated CLI args.
---
Nitpick comments:
In `@emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php`:
- Around line 753-759: The client-side form validation currently checks
contMyMAC format but doesn't prevent submission when Extra Params already
specify networking; add a parity check in the same prepareConfig(...) client
path: read the form's contExtraParams (or equivalent extra params input) and if
contMyMAC is non-empty and contExtraParams contains any network option (e.g.
“--network” or “--net”/“-p” style network settings or a literal “host”), show
the same swal error message and return false so the user gets immediate feedback
instead of a server round-trip. Ensure you reference the existing contMyMAC
handling and place this guard before form submission in prepareConfig(...).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0d0decb6-25be-44e3-a4e1-d129f76b1639
📒 Files selected for processing (3)
emhttp/languages/en_US/helptext.txtemhttp/plugins/dynamix.docker.manager/include/CreateDocker.phpemhttp/plugins/dynamix.docker.manager/include/Helpers.php
🚧 Files skipped from review as they are similar to previous changes (2)
- emhttp/languages/en_US/helptext.txt
- emhttp/plugins/dynamix.docker.manager/include/Helpers.php
Purpose of the change: - Keep Fixed MAC validation consistent when Extra Parameters specify Docker networking. - Catch both equals and space-separated Docker CLI network argument forms. How behavior was before: - The server-side guard only detected --net= and --network=. - The client-side form only validated MAC format and allowed a Fixed MAC with Extra Parameters networking until the server rejected it. Why that was a problem: - Users could bypass the Fixed MAC conflict guard with --net bridge or --network br0. - The browser allowed submissions that were known to fail server validation. What the new change accomplishes: - Detects --net and --network followed by either equals or whitespace. - Adds matching client-side feedback before form submission. - Updates the related container-network ExtraParams handling to recognize space-separated container network syntax. How it works: - Centralizes PHP network option detection in hasNetworkParam(). - Mirrors the same detection in the Docker form JavaScript. - Expands container: network parsing and replacement to accept both --network=container:name and --network container:name.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php`:
- Around line 79-84: The existing validation only checks $submittedMAC from
contMyMAC or extractMacAddressParam($_POST['contExtraParams']) but misses
mac-address inside a --network=... entry; update CreateDocker.php to also parse
contExtraParams for mac-address= inside any --network parameter (e.g. detect
patterns like --network=...,mac-address=... or network=name=...,mac-address=...)
and feed that value through isValidUnicastMacAddress (reusing or extending
extractMacAddressParam). If a mac is found in contExtraParams (regardless of
whether contMyMAC was supplied) and is invalid, show the same error UI and goto
END; ensure you reference $submittedMAC, contExtraParams, extractMacAddressParam
and isValidUnicastMacAddress when making the change.
In `@emhttp/plugins/dynamix.docker.manager/include/Helpers.php`:
- Around line 35-53: The current implementations of extractMacAddressParam,
removeMacAddressParam, and hasNetworkParam incorrectly parse flags inside
quoted/escaped values; replace the regex-only approach with shell-aware
tokenization: split ExtraParams into tokens honoring single/double quotes and
escapes, then iterate tokens to find flags (--mac-address, --net/--network) and
extract/remove them without touching quoted content; update
extractMacAddressParam to return the unquoted flag value from the matching
token, removeMacAddressParam to rebuild the params string omitting only the
matching token(s), and hasNetworkParam to check tokens for --net/--network
presence; modify the functions named extractMacAddressParam,
removeMacAddressParam, and hasNetworkParam accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2bd5afe1-f1eb-44df-b7bf-8484af6d6740
📒 Files selected for processing (2)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.phpemhttp/plugins/dynamix.docker.manager/include/Helpers.php
Purpose of the change: - Remove duplicated Docker container form error rendering. How behavior was before: - Fixed MAC validation paths each repeated log output, error markup, and Back button rendering. Why that was a problem: - The duplicated blocks made the validation code noisier and easier to drift as more validation is added. What the new change accomplishes: - Adds a shared dockerFormError() helper for early form validation failures. - Keeps the existing error messages and navigation behavior unchanged. How it works: - The helper renders the standard Docker log frame, translated error label, message, and Back button. - Existing Fixed MAC guards call the helper before jumping to END.
Purpose of the change: - Allow users to set a fixed MAC address when Docker Network Type is Bridge. How behavior was before: - The Fixed MAC address field was inside the Fixed IP section. - Bridge mode hides Fixed IP, so the MAC field was hidden even though Docker accepts endpoint MAC settings for bridge networking. Why that was a problem: - Users could only access the fixed MAC field for custom networks. - Bridge containers could still be created with a fixed endpoint MAC by command generation, but the GUI did not expose the field. What the new change accomplishes: - Splits Fixed MAC address into its own visibility block. - Shows Fixed MAC address for Bridge and custom networks. - Keeps Fixed IP hidden for Bridge and keeps both fields hidden for Host, None, and Container network modes. How it works: - showSubnet() now controls .myIP and .myMAC independently. - Bridge hides and clears Fixed IP while showing Fixed MAC.
Purpose of the change: - Remove custom preflight errors for advanced Fixed MAC edge cases. How behavior was before: - The form blocked Fixed MAC submissions when Extra Parameters also specified networking. - The form also pre-validated unicast MAC semantics before Docker ran. Why that was a problem: - The duplicate network case requires an advanced self-conflicting setup and Docker already reports it clearly. - Invalid MAC assignment errors are also surfaced by Docker during command execution. - The extra custom validation added code and UX paths that were not necessary for the main feature. What the new change accomplishes: - Keeps the Fixed MAC field and endpoint command generation focused. - Removes custom server and client preflight validation for duplicate network args and invalid unicast MACs. - Keeps lightweight input normalization for valid MAC formats. How it works: - prepareConfig() still normalizes hyphenated MAC input to colon-separated lowercase form. - Docker command execution remains the authority for invalid MAC values or duplicate network flags.
Purpose of the change: - Avoid treating quoted or escaped Extra Parameters values as Docker network or MAC flags. How behavior was before: - Helper functions used regexes directly on the full Extra Parameters string. - Text such as --network or --mac-address inside quoted values could be detected as real flags. Why that was a problem: - Template migration could extract or remove a MAC flag from quoted content that was not an actual Docker CLI option. - Network ownership detection could incorrectly treat quoted values as --net/--network options. What the new change accomplishes: - Adds shell-aware tokenization for Extra Parameters. - Finds --mac-address and --net/--network only when they appear as actual tokens. - Removes only the real MAC flag token and its value while preserving quoted content. How it works: - Tokenization tracks single quotes, double quotes, and escapes. - extractMacAddressParam(), removeMacAddressParam(), and hasNetworkParam() now operate on parsed token values instead of raw regex matches.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@emhttp/plugins/dynamix.docker.manager/include/Helpers.php`:
- Around line 479-481: The IPv6 detection using strpos($myIP,':') relies on
truthiness and fails for addresses that begin with '::' (strpos returns 0);
update the condition in the block that builds $cmdMyIP (where $xml['MyIP'] is
exploded and $cmdMyIP appended with '--ip=' or '--ip6=') to test strpos($myIP,
':') !== false (or an equivalent explicit boolean check) so addresses with ':'
at position 0 are correctly treated as IPv6.
- Around line 470-472: The IPv6 detection incorrectly treats addresses starting
with ':' as IPv4 because strpos($myIP, ':') returns 0 which is falsy; update the
check in the loop that builds $networkEndpoint (the foreach over explode and
$myIP handling in Helpers.php) to test strpos($myIP, ':') !== false (or use a
function like preg_match('/:/', $myIP)) so any string containing a colon is
classified as IPv6 and prepended with 'ip6='; keep the existing empty-string
gate (if ($myIP)) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e017f461-eead-4c96-8c25-b0cb751a44a1
📒 Files selected for processing (2)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.phpemhttp/plugins/dynamix.docker.manager/include/Helpers.php
🚧 Files skipped from review as they are similar to previous changes (1)
- emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
- Replace the shell-style Extra Parameters tokenizer with a smaller conservative parser. - Before, the helper attempted to fully tokenize Extra Parameters to migrate legacy --mac-address values and detect --network/--net ownership. - That was more complex than this migration path needs and increased maintenance risk around shell quoting edge cases. - Now, only simple top-level unquoted --mac-address, --network, and --net forms are interpreted by the migration helpers. - Quoted values are masked or preserved so labels and environment values containing those strings are left untouched.
- Update Docker command generation to test IPv6 detection with an explicit strpos comparison. - Before, addresses starting with :: produced strpos(..., ':') === 0 and were treated as IPv4 because 0 is falsy. - That could emit ip= or --ip= for IPv6 values that begin with a colon. - Now, both endpoint-level network options and legacy --ip/--ip6 flags classify any address containing a colon as IPv6. - The existing empty-address guard remains unchanged.
- Reconnect Docker network endpoints when a stored fixed MAC differs from the current endpoint MAC. - Before, the network restore path returned as soon as an endpoint existed, so Docker daemon restarts could keep a newly randomized MAC even though the template stored MyMAC. - That meant a dummy container edit recreated the container with the right MAC, but normal Docker restart did not repair the existing endpoint. - Now, restart restore keeps the fast path when the endpoint is absent, has no stored MAC, or already matches, and only disconnects/reconnects when the stored MAC needs to be restored. - This applies to custom network names such as eth0 as well as br0 because it runs through the same netrestore path.
- Switch the Docker network restore path from endpoint driver options to the Docker network connect API when a fixed MAC is stored. - Before, rc.docker passed com.docker.network.endpoint.macaddress through docker network connect --driver-opt. - Docker 29.3.1 persisted that value in DriverOpts but still assigned a random endpoint MacAddress on macvlan networks. - That meant containers could reboot with random MACs even though MyMAC was stored in the template. - Now, fixed-MAC reconnects send EndpointConfig.MacAddress, with IPv4 and IPv6 addresses preserved through IPAMConfig, matching the working docker run endpoint form.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@emhttp/plugins/dynamix.docker.manager/include/Helpers.php`:
- Around line 82-92: normalizeMacAddress() currently returns an empty string for
any non-normalizable MAC which causes postToXML() and xmlToVar() to silently
drop user input; change normalizeMacAddress($mac) to return the original trimmed
input (not '') when the value fails validation so the raw value is preserved,
and update callers postToXML() and xmlToVar() (the logic around persisting the
Fixed MAC field) to use the raw input when normalizeMacAddress indicates invalid
(i.e., do not overwrite user input with an empty string) so Docker can perform
its own rejection; ensure the function still normalizes valid 12-hex or dash
formats into colon-separated lowercase and only returns '' for an originally
empty/blank input.
- Around line 399-411: The code lowercases $xml['Network'] into $networkName and
then uses it when emitting Docker CLI args; Docker is case-sensitive so preserve
the original network name. Change the logic so you keep the original name (e.g.,
$origNetwork = $xml['Network']) and only use a lowercased copy (e.g.,
$networkCheck = strtolower($origNetwork)) for the special-case check against
['host','none']; then use the original name ($origNetwork) when building
$networkEndpoint, setting 'name=...', and when assigning $cmdNetwork for both
the '--network=' and '--net=' branches while keeping the existing
removeMacAddressParam, $networkEndpoint assembly, and escapeshellarg usage
intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5806865b-2220-48b4-926b-4cfac09e6bb5
📒 Files selected for processing (3)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.phpemhttp/plugins/dynamix.docker.manager/include/Helpers.phpetc/rc.d/rc.docker
🚧 Files skipped from review as they are similar to previous changes (2)
- etc/rc.d/rc.docker
- emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
Comment on lines
+82
to
+92
| function normalizeMacAddress($mac) { | ||
| $mac = strtolower(trim($mac ?? '')); | ||
| if ($mac === '') { | ||
| return ''; | ||
| } | ||
| if (preg_match('/^[0-9a-f]{12}$/', $mac)) { | ||
| $mac = implode(':', str_split($mac, 2)); | ||
| } else { | ||
| $mac = str_replace('-', ':', $mac); | ||
| } | ||
| return preg_match('/^([0-9a-f]{2}:){5}[0-9a-f]{2}$/', $mac) ? $mac : ''; |
Contributor
There was a problem hiding this comment.
Don't silently drop invalid Fixed MAC input.
normalizeMacAddress() collapses any non-normalizable value to '', and both postToXML() and xmlToVar() persist only that normalized result. If a user types an invalid MAC in the new field, the template quietly clears it instead of preserving the value and letting Docker reject the create. That loses user input and bypasses the intended Docker-owned error path.
Proposed fix
- $myMAC = $extraNetwork ? '' : normalizeMacAddress(trim($post['contMyMAC'] ?? '') ?: extractMacAddressParam($post['contExtraParams'] ?? ''));
+ $rawMyMAC = trim($post['contMyMAC'] ?? '');
+ $legacyMyMAC = extractMacAddressParam($post['contExtraParams'] ?? '');
+ $myMAC = $extraNetwork ? '' : ($rawMyMAC !== '' ? (normalizeMacAddress($rawMyMAC) ?: $rawMyMAC) : normalizeMacAddress($legacyMyMAC));
$xml->MyMAC = xml_encode($myMAC);- $out['MyMAC'] = $extraNetwork ? '' : normalizeMacAddress(xml_decode($xml->MyMAC ?? '') ?: extractMacAddressParam($extraParams));
+ $storedMyMAC = xml_decode($xml->MyMAC ?? '');
+ $out['MyMAC'] = $extraNetwork ? '' : ($storedMyMAC !== '' ? (normalizeMacAddress($storedMyMAC) ?: $storedMyMAC) : normalizeMacAddress(extractMacAddressParam($extraParams)));Also applies to: 138-151, 215-228
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@emhttp/plugins/dynamix.docker.manager/include/Helpers.php` around lines 82 -
92, normalizeMacAddress() currently returns an empty string for any
non-normalizable MAC which causes postToXML() and xmlToVar() to silently drop
user input; change normalizeMacAddress($mac) to return the original trimmed
input (not '') when the value fails validation so the raw value is preserved,
and update callers postToXML() and xmlToVar() (the logic around persisting the
Fixed MAC field) to use the raw input when normalizeMacAddress indicates invalid
(i.e., do not overwrite user input with an empty string) so Docker can perform
its own rejection; ensure the function still normalizes valid 12-hex or dash
formats into colon-separated lowercase and only returns '' for an originally
empty/blank input.
Comment on lines
+399
to
+411
| $networkName = strtolower($xml['Network']); | ||
| if ($extraNetwork) { | ||
| $cmdNetwork = ""; | ||
| } elseif (strlen($xml['MyMAC']) && !in_array($networkName, ['host','none'])) { | ||
| $xml['ExtraParams'] = removeMacAddressParam($xml['ExtraParams']); | ||
| $networkEndpoint = ['name='.$networkName]; | ||
| foreach (explode(' ',str_replace(',',' ',$xml['MyIP'])) as $myIP) { | ||
| if ($myIP) $networkEndpoint[] = (strpos($myIP,':') !== false ? 'ip6=' : 'ip=').$myIP; | ||
| } | ||
| $networkEndpoint[] = 'mac-address='.$xml['MyMAC']; | ||
| $cmdNetwork = '--network='.escapeshellarg(implode(',', $networkEndpoint)); | ||
| } else { | ||
| $cmdNetwork = '--net='.escapeshellarg($networkName); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Are mixed-case Docker network names valid in Docker Engine, and are they matched case-sensitively when passed through docker create --network/docker run --network?
💡 Result:
Yes, mixed-case Docker network names are valid in Docker Engine. They are matched case-sensitively when passed through docker create --network or docker run --network. Docker network names have minimal validation restrictions compared to container or volume names. Unlike containers/volumes which follow a strict regex [a-zA-Z0-9][a-zA-Z0-9_.-]*, networks historically allowed broader characters (e.g., %, ^) as long as non-empty, though later versions tightened some rules for consistency. Official docs do not prohibit uppercase letters, and community reports confirm mixed-case names like "ThIsIsANeTWoRk" can be created successfully. Network matching by name in --network is case-sensitive because Docker stores and lists network names preserving case (e.g., docker network ls shows exact case), and Linux filesystems (where Docker stores network config under /var/lib/docker/network/files/local) are case-sensitive. When specifying a partial ID, Docker resolves it to the name first, then matches, confirming case-exact name lookup. No sources indicate case-insensitive name matching for networks; contrasts with container DNS resolution (which became case-insensitive in later versions via libnetwork changes).
Citations:
- 1: docker network name validation is not so strict moby/moby#27449
- 2: https://forums.unraid.net/bug-reports/stable-releases/6123-case-sensitive-docker-network-name-r2680/
- 3: Specifying
docker run --network <network-id>attempts to match on name moby/moby#41520 - 4: https://stackoverflow.com/questions/42642561/docker-restrictions-regarding-naming-container
- 5: NameRegex used for network does not match docker containers/podman#18263
- 6: https://docs.docker.com/reference/cli/docker/network/ls
🏁 Script executed:
# First, let's check the file and see the full context of this function
cd /root && find . -type f -name "Helpers.php" | head -20Repository: unraid/webgui
Length of output: 112
🏁 Script executed:
# Examine the file with line numbers, showing context around the target lines
if [ -f "./emhttp/plugins/dynamix.docker.manager/include/Helpers.php" ]; then
wc -l "./emhttp/plugins/dynamix.docker.manager/include/Helpers.php"
fiRepository: unraid/webgui
Length of output: 120
🏁 Script executed:
# Get the full function context - find the function this code belongs to
rg -B 30 'strtolower\(\$xml\[.Network.\]\)' --context 3 ./emhttp/plugins/dynamix.docker.manager/include/Helpers.php | head -80Repository: unraid/webgui
Length of output: 387
🏁 Script executed:
# Check how $xml['Network'] is used elsewhere in the file before this point
rg '\$xml\[.Network.\]' --context 2 ./emhttp/plugins/dynamix.docker.manager/include/Helpers.phpRepository: unraid/webgui
Length of output: 804
🏁 Script executed:
# Get broader context of the function - search backwards to find the function signature
sed -n '350,420p' ./emhttp/plugins/dynamix.docker.manager/include/Helpers.php | cat -nRepository: unraid/webgui
Length of output: 3339
🏁 Script executed:
# Look for the function name and its start
rg 'function.*\{' -B 5 ./emhttp/plugins/dynamix.docker.manager/include/Helpers.php | grep -A 10 'xmlToCommand'Repository: unraid/webgui
Length of output: 1301
🏁 Script executed:
# Get the complete section around lines 390-420 to see the full logic
sed -n '390,420p' ./emhttp/plugins/dynamix.docker.manager/include/Helpers.php | cat -nRepository: unraid/webgui
Length of output: 1893
Preserve the configured network name when emitting CLI arguments.
The code lowercases $xml['Network'] unconditionally at line 399, then uses that lowercased name in network endpoint definitions and --net= / --network= commands. Docker matches network names case-sensitively, so a custom network named "MyNetwork" would fail when the code passes "mynetwork" to Docker.
The strtolower() is only needed for the case-insensitive check against the special modes ['host','none'] at line 407. The original network name should be preserved for command emission.
Proposed fix
- $networkName = strtolower($xml['Network']);
+ $networkName = $xml['Network'];
+ $networkMode = strtolower($networkName);
if ($extraNetwork) {
$cmdNetwork = "";
- } elseif (strlen($xml['MyMAC']) && !in_array($networkName, ['host','none'])) {
+ } elseif (strlen($xml['MyMAC']) && !in_array($networkMode, ['host','none'])) {
$xml['ExtraParams'] = removeMacAddressParam($xml['ExtraParams']);
$networkEndpoint = ['name='.$networkName];
foreach (explode(' ',str_replace(',',' ',$xml['MyIP'])) as $myIP) {
if ($myIP) $networkEndpoint[] = (strpos($myIP,':') !== false ? 'ip6=' : 'ip=').$myIP;
}
$networkEndpoint[] = 'mac-address='.$xml['MyMAC'];
$cmdNetwork = '--network='.escapeshellarg(implode(',', $networkEndpoint));
} else {
$cmdNetwork = '--net='.escapeshellarg($networkName);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@emhttp/plugins/dynamix.docker.manager/include/Helpers.php` around lines 399 -
411, The code lowercases $xml['Network'] into $networkName and then uses it when
emitting Docker CLI args; Docker is case-sensitive so preserve the original
network name. Change the logic so you keep the original name (e.g., $origNetwork
= $xml['Network']) and only use a lowercased copy (e.g., $networkCheck =
strtolower($origNetwork)) for the special-case check against ['host','none'];
then use the original name ($origNetwork) when building $networkEndpoint,
setting 'name=...', and when assigning $cmdNetwork for both the '--network=' and
'--net=' branches while keeping the existing removeMacAddressParam,
$networkEndpoint assembly, and escapeshellarg usage intact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--network=name=...,ip=...,mac-address=...syntax when the GUI owns the network setting.--mac-addressvalues out of Extra Parameters while preserving ExtraParams-owned--network/--netcases.--mac-address,--network, and--netforms are interpreted, while quoted values are left untouched.EndpointConfig.MacAddressfor fixed-MAC restores, because Docker 29.3.1 stores but does not apply the equivalent CLI driver option on macvlan.--network=br0and--network br0.::.Verification
php -d short_open_tag=1 -l emhttp/plugins/dynamix.docker.manager/include/Helpers.phpphp -d short_open_tag=1 -l emhttp/plugins/dynamix.docker.manager/include/CreateDocker.phpbash -n etc/rc.d/rc.dockergit diff --check--network='name=bridge,mac-address=02:42:9a:0d:7e:c0'.02-42-9A-0D-7E-C0normalizes to02:42:9a:0d:7e:c0.--network=br0,--network br0,--net=host, and--net hostin PHP paths.--env "FOO=bar --network br0"and--label '--mac-address 11:22:33:44:55:66'untouched.::1emitsip6=::1for endpoint options and--ip6='::1'for legacy IP flags.bash -n etc/rc.d/rc.docker; it now compares current endpoint MACs before returning and reconnects only when the stored MAC differs.root@192.168.0.206: CLI--driver-opt=com.docker.network.endpoint.macaddress=...persisted inDriverOptsbut still produced a random endpoint MAC./networks/br0/connectwithEndpointConfig.MacAddressrestores the live Firefox endpoint MAC to02:42:9a:0d:7e:c0.root@192.168.0.206with Firefox in autostart; after boot, Firefox was running onbr0with template MAC02:42:9a:0d:7e:c0, live endpoint MAC02:42:9a:0d:7e:c0, and IP192.168.0.6.Summary by CodeRabbit